Conversation
anarchivist
left a comment
anarchivist
left a comment
There was a problem hiding this comment.
I think this needs some refactoring - we should look at using ruby-jwt's native methods here rather than rolling our own.
There was a problem hiding this comment.
Similarly, there's a lot in this file that seems to duplicate what's in JWT::JWK's methods here.
danschmidt5189
left a comment
danschmidt5189
left a comment
There was a problem hiding this comment.
I have a few nits but they're nothing major (see comments).
One bigger picture question is whether to implement this using an AlmaJwtValidator module (as you've done) or an AlmaJwt class. The benefit of the class is that it could naturally encapsulate the accessor method you implicitly need (username -> alma_id). Not a blocker, but something to think about from an implementation / code style perspective.
| require 'json' | ||
|
|
||
| module AlmaJwtValidator | ||
| JWKS_URL = 'https://api-na.hosted.exlibrisgroup.com/auth/01UCS_BER/jwks.json'.freeze |
There was a problem hiding this comment.
Does the sandbox publish a different URL? If so we'd need to be able to inject this (e.g. via ENV).
There was a problem hiding this comment.
As we noticed when pairing, the actual issuer is Prima. From our testing:
# Get a Ruby shell with jwt installed...
$ docker run --rm -it ruby:3 bash -c 'gem install pry; gem install jwt; exec pry'
# Login to Alma, then visit the fees link from your account page.
# Extract the jwt from the URL in the Framework redirect and decode it.
require 'jwt'
jwt = "{copy from URL}"
JWT.decode(jwt, nil, false)|
|
||
| module AlmaJwtValidator | ||
| JWKS_URL = 'https://api-na.hosted.exlibrisgroup.com/auth/01UCS_BER/jwks.json'.freeze | ||
| EXPECTED_ISS = 'https://api-na.hosted.exlibrisgroup.com/auth/01UCS_BER'.freeze |
| jwks: jwk_set | ||
| } | ||
|
|
||
| JWT.decode(token, nil, true, options) |
There was a problem hiding this comment.
I suppose this wasn't clear from my earlier comment, but I'm curious about why we're using JWT.decode instead of instantiating JWT::EncodedToken and using its methods.
4 participants
No description provided.